New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hive Ranger security plugin #15519

Closed

ashishtadose wants to merge 5 commits into prestodb:master from ashishtadose:hive-ranger-plugin

Contributor

ashishtadose commented Dec 14, 2020

Test plan - (Please fill in how you tested your changes)

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines. Don't forget to follow our attribution guidelines for any code copied from other projects.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== RELEASE NOTES ==

General Changes
* ...
* ...

Hive Changes
* ...
* ...

If release note is NOT required, use:

== NO RELEASE NOTE ==

Contributor Author

ashishtadose commented Dec 14, 2020

resolves #8980

dborkar commented Dec 22, 2020

@nezihyigitbasi can you please help prioritize this? many users are interested in the plugin. Who'd be best to review?

mayankgarg1990 suggested changes

View reviewed changes

Contributor

mayankgarg1990 left a comment

Took a first pass mainly on RangerBasedAccessControl and RangerAuthorizer class

...e/src/main/java/com/facebook/presto/hive/security/ranger/RangerBasedAccessControlConfig.java Outdated Show resolved Hide resolved

...e/src/main/java/com/facebook/presto/hive/security/ranger/RangerBasedAccessControlConfig.java Outdated Show resolved Hide resolved

...e/src/main/java/com/facebook/presto/hive/security/ranger/RangerBasedAccessControlConfig.java Outdated Show resolved Hide resolved

presto-hive/src/main/java/com/facebook/presto/hive/security/ranger/VXUser.java Outdated Show resolved Hide resolved

presto-hive/src/main/java/com/facebook/presto/hive/security/ranger/VXUser.java Show resolved Hide resolved

...to-hive/src/main/java/com/facebook/presto/hive/security/ranger/RangerBasedAccessControl.java Outdated

    
                  {

                      SchemaTableName schemaTableName = new SchemaTableName(schemaName, schemaName);

                      if (!checkAccess(identity, schemaTableName, null, HiveAccessType.DROP)) {

                          denyCreateSchema(schemaTableName.toString(), format("Access denied - User [ $s ] does not have is not have [DROP] " +

Contributor

mayankgarg1990 Dec 22, 2020

for the schema name argument - you should just say schemaName instead of schemeTableName.toString()

Contributor Author

ashishtadose Dec 23, 2020

ok

...to-hive/src/main/java/com/facebook/presto/hive/security/ranger/RangerBasedAccessControl.java Outdated

    
                   */

                  public void checkCanDropSchema(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, String schemaName)

                  {

                      SchemaTableName schemaTableName = new SchemaTableName(schemaName, schemaName);

Contributor

mayankgarg1990 Dec 22, 2020

Lets assume that the schema in question here is s1 - I don't have previous experience with Ranger - what we want to check here is that if the user has permission to drop schema s1, but given this implementation - how will it differentiate between what we want and the permission to drop table s1 in schema s1

Contributor Author

ashishtadose Dec 23, 2020

This implementation is keeping authorization checks similar to Hive since it is verifying against policies set for Hive service.
Ranger authorization checks are validated and inherited from schema, table & column.
Ranger policy can define if accesses are across all tables in the schema or specific table. The policy can allow/restrict user or group for drop access across which will be leveraged for schema and then more specific & restrictive policies for a table can override that behavior.
Policies are evaluated in a specific order to ensure predictable results.
More details here - https://cwiki.apache.org/confluence/display/RANGER/Deny-conditions+and+excludes+in+Ranger+policies
TestRangerBasedAccessControl has concrete tests around this.

...to-hive/src/main/java/com/facebook/presto/hive/security/ranger/RangerBasedAccessControl.java Outdated

Comment on lines 256 to 300

    
                          denyCreateTable(tableName.toString(), format("Access denied - User [ $s ] does not have is not have [CREATE] " +

                                  "privilege on [ %s ] ", identity.getUser(), tableName.getSchemaName()));

Contributor

mayankgarg1990 Dec 22, 2020

Fix extraInfo as per suggestion above and please do the same for all instances below as well

...to-hive/src/main/java/com/facebook/presto/hive/security/ranger/RangerBasedAccessControl.java Outdated

    
                          }

                      }

                      if (deniedColumns.size() > 0) {

                          denySelectColumns(tableName.toString(), deniedColumns, format("Access denied - User [ $s ] does not have is not have [ALTER] " +

Contributor

mayankgarg1990 Dec 22, 2020

lets edit the error message to be in sync with what I stated above and also mention the columns that the user does not have access to

Contributor Author

ashishtadose Dec 23, 2020

Changing tableName.toString() to tableName.getTableName() as not all callback methods provides string schemaName arg but provides SchemaTableName wrapper class

...to-hive/src/main/java/com/facebook/presto/hive/security/ranger/RangerBasedAccessControl.java

    
                  @Override

                  public void checkCanCreateView(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName viewName)

                  {

                      if (!checkAccess(identity, null, null, HiveAccessType.CREATE)) {

Contributor

mayankgarg1990 Dec 22, 2020

should we be passing the schema to check for permission within that ?

Contributor Author

ashishtadose Dec 23, 2020

For underline table and column access checks beyond the view individual access callback are triggered to verify.

Contributor

mayankgarg1990 commented Dec 22, 2020

@ashishtadose - in addition to the code comments above - can you also comment how did you test this PR other than the unittests added in this PR - did you get a chance to run it against an actual Ranger instance and ensure that the code works.

Also, can you update the PR description with the appropriate release notes. https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines has guidelines on how to format your release notes.

Contributor Author

ashishtadose commented Dec 23, 2020

@mayankgarg1990 thanks for the review.
This plugin's functional verification has been carried with a distributed 2.6.x hadoop cluster with Ranger 0.7.x.
I'm planning to certify this for ranger 1.x as well with this PR.

Will accommodate review comments and update the PR.

sajjoseph commented Dec 28, 2020

Good job @ashishtadose.
Multiple organizations have Hadoop 3.x with Ranger 2.x installations and so it will be great if you can get this plugin working with those combinations. Column masking, row filtering and tag support are essential as well.

ashishtadose force-pushed the hive-ranger-plugin branch from 2a8a356 to bc5814c Compare

December 30, 2020 06:51

dborkar commented Jan 25, 2021

@sajjoseph You may be interested in the Ranger + PrestoDB meetup coming up. https://www.meetup.com/prestodb/events/275832071/

agrawalreetika mentioned this pull request

how to support ranger #15452

Open

discipleforteen commented Feb 18, 2021 •

edited

Loading

will it work for Column masking, row filtering and tag support in Ranger 2.1?

ashishtadose requested a review from mayankgarg1990

April 20, 2021 05:57

dborkar commented Apr 25, 2021

@mayankgarg1990 Can you please review? - there is very high interest in the community (via github issues and slack) for this plugin and we'd like to get it merged in as soon as possible.

rohanpednekar requested a review from aweisberg

June 3, 2021 02:59

Contributor

rohanpednekar commented Jun 3, 2021

@mayankgarg1990 Do you think you can review this PR?
@aweisberg and @tdcmeehan, requesting your help to get this prioritized for review.

rohanpednekar added the Roadmap label

linux-foundation-easycla bot commented Jun 28, 2021 •

edited

Loading

The committers are authorized under a signed CLA.

✅ Ashish (bc5814c99a2ce66d9567d2bc00ba1775e4440bd7)

ashishtadose force-pushed the hive-ranger-plugin branch from 4dc1363 to bc5814c Compare

June 28, 2021 11:48

ashishtadose closed this

ashishtadose force-pushed the hive-ranger-plugin branch from 33546cd to cd48fd9 Compare

June 28, 2021 11:52

ashishtadose added 5 commits

June 28, 2021 17:27


          Hive Ranger security plugin

142cdaa


          Upgrade ranger to 2.1

ab93f1c


          Support to access Ranger REST resources on secured ssl server

888f364


          Support to provide authorization based upon Roles defined in Ranger

d30bbb3


          Doc update

6c9cb68

ashishtadose reopened this

Contributor Author

ashishtadose commented Jun 28, 2021

Have rebased this PR and also updated it with the below features.

Support for Ranger 2.1.x
Integration with SSL secured Ranger services
Ranger role-based access authorization.

Contributor Author

ashishtadose commented Jun 28, 2021

@mayankgarg1990 can you help in reviewing this. Thanks

rohanpednekar mentioned this pull request

Feature Request: Ranger authorization integration #8980

Open

rubenssoto commented Aug 23, 2021

Hello,

Any updates on it? Great and important feature

@ashishtadose
@rohanpednekar

rubenssoto mentioned this pull request

unable to integrate ranger with presto #16655

Open

rubenssoto commented Sep 28, 2021

Hello People,

Do you have any updates on this PR? This feature is very important for us

rohanpednekar mentioned this pull request

Presto ranger integration with row-level filters and column masking #11640

Closed

Contributor

rohanpednekar commented Oct 1, 2021

@mayankgarg1990 this is coming up more. Could you please help up to review this PR?

CC @jbapple @aweisberg

Contributor

rohanpednekar commented Nov 10, 2021

Hi @tdcmeehan, Do you think we can get some help here, targetting to get this PR merged by PrestoCon. Thanks!

Contributor

mayankgarg1990 commented Nov 10, 2021

Lets bring this onto master and then I can review this PR

Contributor

Ayan07 commented Nov 11, 2021

@mayankgarg1990 can the merge process be expedited?

Contributor

mayankgarg1990 commented Nov 11, 2021

@Ayan07 - the PR needs to be reviewed and is currently based on a 5 month old base. I am willing to prioritize this PR review - but it will be a combination of PR review and response to the review comments. Before I start to review I want to ensure that the PR is on the latest master so that reviewing and testing it is easy

Contributor Author

ashishtadose commented Nov 14, 2021

@mayankgarg1990 agree this PR needs to be updated with base and would be good to be enhanced with audit support.
I won't be able to spend more time on this anytime soon, @agrawalreetika would you be able to take this up and maybe raise a separate PR for this.

Member

agrawalreetika commented Nov 15, 2021

@mayankgarg1990 agree this PR needs to be updated with base and would be good to be enhanced with audit support. I won't be able to spend more time on this anytime soon, @agrawalreetika would you be able to take this up and maybe raise a separate PR for this.

Sure @ashishtadose I can raise a new PR along with new changes which are not there in this.

agrawalreetika mentioned this pull request

Presto hive Ranger plugin #16999

Merged

Member

agrawalreetika commented Nov 16, 2021

Lets bring this onto master and then I can review this PR

@mayankgarg1990 I have raised a new PR along with new changes #16999 . Please review that.

Contributor

rohanpednekar commented Apr 26, 2022

Refer #16999 for further work.

rohanpednekar closed this

hmadison mentioned this pull request

Global Row Level Security Support for Presto #20572

Open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Roadmap